Address several warnings raised during tests #1351

wshanks · 2023-12-20T15:54:22Z

This change rolls together several small changes to avoid warnings generated when running the tests.

Here is a summary of the changes:

Update use of datetime.utcnow() deprecated in Python 3.12
Concatenate pandas dataframes more carefully to avoid FutureWarning when concatenating an empty dataframe (see DEPR: concat ignoring empty objects pandas-dev/pandas#52532).
Adjust plotter tests to avoid warnings
- Use supported options rather than arbitrary options to avoid unexpected option warning
- Catch expected warning when passing an untrained discriminator to IQPlotter
Move the setting of the axis scale earlier in the logic of MplDrawer.format_canvas. format_canvas queries the limits of each axes object and uses those values. When no data set on a set of axes (an odd edge case), the default values depend on the scale (because the minimum for linear is 0 but for log is 0.1), so the scale needs to be set first.
Remove transpile options from FineDragCal (_transpiled_circuits for BaseCalibrationExperiment warns about and ignores transpile options)
Replace isinstance checks with pulse_type checks for calibration tests checking pulse shape
Replace qiskit.Aer usage with qiskit_aer
Set tolerance on cvxpy test to larger value, close to the tolerance achieved in practice. The routine was maxing out its iterations without achieving tolerance but still producing a close enough result for the test to pass. Increasing the tolerance avoids the max iterations warning and makes the test faster.
Rename TestLibrary to MutableTestLibrary. The class name starting with Test* causes pytest to warn about the class looking like a class holding test cases. pytest is not the main way we run the tests but it still seems best to avoid confusion between test case classes and test helper classes.
Catch warnings about insufficient trials in QuantumVolume tests using small numbers of trials.
Catch user and deprecation warnings about calibration saving and loading to csv files.
Convert existing calibration saving and loading tests from json to csv, leaving basic coverage of csv saving. A bug was found with json saving, resulting in one test being marked as an expected failure.
Set data on the ExperimentData for TestFramework.test_run_analysis_experiment_data_pickle_roundtrip to avoid a warning about an empty experiment data object. Other cases of this warning were addressed in 729014b but this test case was added afterward in a6c9e53.

In order to avoid warnings creeping in in the future, this change also makes any warnings emitted by qiskit-experiments code be treated as exceptions by the tests (with an exception for ScatterTable methods for which it is planned to remove the deprecation). Any expected warnings in tests can be handled by using assertWarns or warnings.catch_warnings.

As all the changes relate to tests (except the FineDragCal one which is a non-functional change to avoid a warning), no release note was added.

See pandas-dev/pandas#52532

Calibration experiments currently override `_transpiled_circuits()` and ignore transpile options so these options were not doing anything besides generating a `UserWarning` when creating a `FineDragCal` instance.

The default tolerance was leading to a warning about max iterations being reached without reaching the target objective.

wshanks · 2023-12-20T16:29:07Z

The only remaining warnings generated by qiskit_experiments when running the tests are the CurveData backwards compatibility methods of ScatterTable. Since the API of ScatterTable is still under discussion, I didn't try to change those uses.

Currently we mark a test as failing if it raises a DeprecationWarning. I was wondering if we should also mark a test as failing if it raises any warning from the qiskit_experiments package. It is hard to control warnings from other packages but we should be able to control our own. If we want to test generating warnings, we can use assertWarns or warnings.catch_warnings to avoid specific cases of warnings failing the tests.

coruscating

Thanks for fixing all these warnings and the changes look good. Besides the QV warnings, there's one remaining from TestFramework.test_run_analysis_experiment_data_pickle_roundtrip: https://github.com/Qiskit-Extensions/qiskit-experiments/actions/runs/7482494338/job/20366191685?pr=1351#step:8:486
And one from TestPlotter .test_series_data_end_to_end: https://github.com/Qiskit-Extensions/qiskit-experiments/actions/runs/7482494338/job/20366191685?pr=1351#step:8:1082

I agree with failing the tests on warnings coming from qiskit_experiments. Do you want to do it in this PR or a new one?

coruscating · 2024-01-11T01:04:50Z

test/visualization/test_plotter_mpldrawer.py

@@ -101,7 +101,11 @@ def test_unit_scale(self, args):
    def test_scale(self):
        """Test the xscale and yscale figure options."""
        plotter = MockPlotter(MplDrawer(), plotting_enabled=True)
-        plotter.set_figure_options(xscale="quadratic", yscale="log")
+        plotter.set_figure_options(


Outside the scope of this PR, but we should probably handle the case of a log scale with limit 0 gracefully.

I decided to address this here (see aed5d30). I was able to find a somewhat elegant solution by moving the setting of the axis scale earlier in the process (see the commit message). The tricky part is that the code wants to know the axis limits for the part where it tries to rescale the format (like add a k and hide the 000) but if you have axes that have no data in them matplotlib returns 0,1 by default. If the code didn't do that formatting scaling, I would just change it to not set limits when no user limits were specified so matplotlib could use its default limits.

When the axis scale is changed to "log", matplotlib changes the default axis limits from (0, 1) to something around (0.1, 10). Since MplDrawer got the limits and the set them back on the axes after setting the scale, it was reapplying a minimum of 0 to the log scale plot, leading to a user warning. matplotlib by default changes the limits to the range of the data when data is added, so this case only came up when generating a figure with no data.

wshanks · 2024-01-11T20:47:33Z

Interesting. I made warnings cause tests to fail and a few other warnings surfaced. I will address them. I wonder why I didn't see them before in the test log when running all the tests to check for warnings.

Also, catch both user and deprecation warnings for remaining csv calibration saving tests.

wshanks · 2024-01-12T22:43:44Z

@coruscating I made the warnings into failures, addressed the extra warnings you mentioned, and handled the 0 case of log scale plots better. Along the way, I found a bug in the calibration save/load code (#1355).

coruscating

Thanks, the axis scale fix is a nice one. We could add a release note for it, but it was unlikely that a user would have encountered these warnings, so it's up to you. Either way this is good to go.

qiskit_experiments/visualization/drawers/mpl_drawer.py

Co-authored-by: Helena Zhang <[email protected]>

wshanks · 2024-01-19T14:22:57Z

By the way, I think we need this now to fix the tests against Qiskit main. It looks like the test run is failing during test discovery now because of the removal of Aer in Qiskit/qiskit#11442.

wshanks added 9 commits December 19, 2023 18:45

Update deprecated datetime usage

a30a012

Avoid pandas FutureWarning on concatenating empty data frame

449634a

See pandas-dev/pandas#52532

Add plotter test options to avoid warnings

db546a4

Remove transpile options from FineDragCal

1babe3a

Calibration experiments currently override `_transpiled_circuits()` and ignore transpile options so these options were not doing anything besides generating a `UserWarning` when creating a `FineDragCal` instance.

Ignore plotter warning in test

dbd35f6

Update deprecated pulse usage in calibration test

4b5239b

Remove deprecated use of qiskit.Aer from tests

7650468

Increase tolerance of cvxpy fitters in tomography test

db4e43b

The default tolerance was leading to a warning about max iterations being reached without reaching the target objective.

Change gate library test to avoid pytest identification

fa1354c

wshanks force-pushed the warnings branch from 6e781b9 to fa1354c Compare December 20, 2023 16:24

wshanks and others added 2 commits December 20, 2023 12:59

Catch expected warnings in quantum volume tests

afe4257

Merge branch 'main' into warnings

d66fba7

coruscating reviewed Jan 11, 2024

View reviewed changes

wshanks added 5 commits January 11, 2024 12:26

Avoid empty experiment data warning for pickle roundtrip test

3e70392

Avoid unexpected data warning in plotter test

9805d50

Make all unexpected warnings from qiskit_experiments test errors

1741ef1

Add exception for deprecated ScatterTable method usage

e9e9d14

wshanks force-pushed the warnings branch from 3ab44d2 to e9e9d14 Compare January 11, 2024 20:25

Convert calibration saving tests to favor json method

be220b2

Also, catch both user and deprecation warnings for remaining csv calibration saving tests.

wshanks mentioned this pull request Jan 12, 2024

Saving and loading calibration data with json format loses does not fully restore non-schedule parameters #1355

Closed

wshanks requested a review from coruscating January 12, 2024 22:43

This was referenced Jan 16, 2024

Add support for saving/loading calibration parameters without schedules #1357

Merged

Add support for Python 3.12 #1359

Merged

coruscating approved these changes Jan 19, 2024

View reviewed changes

qiskit_experiments/visualization/drawers/mpl_drawer.py Outdated Show resolved Hide resolved

Update qiskit_experiments/visualization/drawers/mpl_drawer.py

b677533

Co-authored-by: Helena Zhang <[email protected]>

wshanks enabled auto-merge January 19, 2024 14:18

Merge branch 'main' into warnings

659286e

wshanks added this pull request to the merge queue Jan 19, 2024

Merged via the queue into qiskit-community:main with commit c536026 Jan 19, 2024
10 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Address several warnings raised during tests #1351

Address several warnings raised during tests #1351

wshanks commented Dec 20, 2023 •

edited

Loading

wshanks commented Dec 20, 2023

coruscating left a comment

coruscating Jan 11, 2024

wshanks Jan 11, 2024

wshanks commented Jan 11, 2024

wshanks commented Jan 12, 2024

coruscating left a comment

wshanks commented Jan 19, 2024

Address several warnings raised during tests #1351

Address several warnings raised during tests #1351

Conversation

wshanks commented Dec 20, 2023 • edited Loading

wshanks commented Dec 20, 2023

coruscating left a comment

Choose a reason for hiding this comment

coruscating Jan 11, 2024

Choose a reason for hiding this comment

wshanks Jan 11, 2024

Choose a reason for hiding this comment

wshanks commented Jan 11, 2024

wshanks commented Jan 12, 2024

coruscating left a comment

Choose a reason for hiding this comment

wshanks commented Jan 19, 2024

wshanks commented Dec 20, 2023 •

edited

Loading